Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revision of CMS inclusive DY data #2238

Merged
merged 17 commits into from
Dec 6, 2024
Merged

Revision of CMS inclusive DY data #2238

merged 17 commits into from
Dec 6, 2024

Conversation

achiefa
Copy link
Contributor

@achiefa achiefa commented Dec 5, 2024

This PR addresses the four datasets listed below:

  • CMS_WPWM_7TEV_ELECTRON_ASY
  • CMS_WPWM_7TEV_MUON_ASY
  • CMS_Z0_7TEV_DIMUON_2D
  • CMS_WPWM_8TEV_MUON_Y

These datasets were already re-implemented in the new format by TR. I think the things to do here are:

  1. Check the filter file is working.
  2. Update the process_type with the processes in process_options.
  3. Update the name of the kinematics (and re-run the filter)

See the reports for these datasets before I apply any change: report

@achiefa achiefa requested a review from scarlehoff December 5, 2024 12:20
@achiefa achiefa self-assigned this Dec 5, 2024
@achiefa
Copy link
Contributor Author

achiefa commented Dec 5, 2024

And this is the report after these changes: report. Nothing has changed compared to the previous versions. I think this one is ready for merging. Let's wait for the tests.

@achiefa achiefa force-pushed the rev_CMS_inclusive_DY branch from fec31c7 to a1d5a5a Compare December 5, 2024 13:31
@achiefa
Copy link
Contributor Author

achiefa commented Dec 5, 2024

@scarlehoff I don't understand that error message that I get from the test

Run echo "Changed files: $CHANGED_FILES"
  echo "Changed files: $CHANGED_FILES"
  exit 1
  shell: /usr/bin/bash -e {0}
  env:
    CHANGED_FILES: nnpdf_data/nnpdf_data/commondata/CMS_WPWM_8TEV_MUON/uncertainties.yaml
Changed files: nnpdf_data/nnpdf_data/commondata/CMS_WPWM_8TEV_MUON/uncertainties.yaml
Error: Process completed with exit code 1.

Can you help me?

@scarlehoff
Copy link
Member

It means that you need to use the label again. Basically it means that the bot would get results different than what you commited (@Radonirinaunimi and I were unable to make the -1 signs agree between computers so we decided to take the bot as the ultimate truth)

@achiefa
Copy link
Contributor Author

achiefa commented Dec 5, 2024

(@Radonirinaunimi and I were unable to make the -1 signs agree between computers so we decided to take the bot as the ultimate truth)

How is that possible 🫤?

@scarlehoff
Copy link
Member

I think it is the eigen value decomposition of the covmat in either scipy or numpy that is basically non reproducible (you would need to take the values and post process them), but having the bot generating all data was something we wanted to do anyway.

@achiefa
Copy link
Contributor Author

achiefa commented Dec 5, 2024

Ok, I think now this is solved. Wait for the tests, and then I think it can be merged.

Copy link
Member

@scarlehoff scarlehoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Beside the comment about the kinematics_override this is good to go.

The only other thing that might be interesting to change is the Z0 variable from mass squared to just mass, so that the plots have the mass in the title

image

(right now the value is of the mass due to the kinematics override, but the units are of course mass squared)

Up to you if you want to change it, the information is of course the same, I just feel the non-squared mass is easier to read.

@@ -31,13 +31,13 @@ implemented_observables:
dataset_label: "CMS Drell-Yan 2D 7 TeV 2011"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing missing is that the kinematics_override needs to be set to identity in all of the files, because otherwise validphys will sqrt the masses (see e.g, the kinematic coverage plot, the scales are too low)

image

@scarlehoff scarlehoff added the Done PRs that are done but waiting on something else to merge/approve label Dec 6, 2024
@scarlehoff scarlehoff merged commit 13fda2a into master Dec 6, 2024
6 checks passed
@scarlehoff scarlehoff deleted the rev_CMS_inclusive_DY branch December 6, 2024 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data toolchain Done PRs that are done but waiting on something else to merge/approve regenerate-data
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants